Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
Lunaria Status Overview🌕 This pull request will trigger status changes. Learn moreBy default, every PR changing files present in the Lunaria configuration's You can change this by adding one of the keywords present in the Tracked Files
Warnings reference
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a quadrant comparison chart to the Compare page: a new Vue Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Actionable comments posted: 7
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: be4b4196-422e-437b-89ff-c11f23cf7b05
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (11)
app/components/Compare/FacetQuadrantChart.vueapp/composables/useChartWatermark.tsapp/pages/compare.vueapp/utils/charts.tsapp/utils/compare-quadrant-chart.tsi18n/locales/en.jsoni18n/locales/fr-FR.jsoni18n/schema.jsonpackage.jsontest/nuxt/a11y.spec.tstest/unit/app/utils/compare-quadrant-chart.spec.ts
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/components/Compare/FacetQuadrantChart.vue (1)
113-128: Consider explicit property assignment to avoid spread in map.The static analyser flags spreading inside
.map()as inefficient. SincePackageQuadrantPointhas a known shape, you could destructure and reassign explicitly. However, if the utility may add properties in future that the chart relies on at runtime, keeping the spread preserves forward compatibility.♻️ Optional: explicit property assignment
const dataset = computed<VueUiQuadrantDatasetItem[]>(() => { - return rawQuadrant.value.map((el: PackageQuadrantPoint) => { - return { - ...el, - fullname: el.name, - name: applyEllipsis(el.name, 20), - shape: 'circle', - color: isListedFramework(el.name) ? getFrameworkColor(el.name) : undefined, - series: [ - { - name: applyEllipsis(el.name, 20), - x: el.x, - y: el.y, - }, - ], - } + return rawQuadrant.value.map((el: PackageQuadrantPoint) => ({ + x: el.x, + y: el.y, + quadrant: el.quadrant, + category: el.category, + fullname: el.name, + name: applyEllipsis(el.name, 20), + shape: 'circle' as const, + color: isListedFramework(el.name) ? getFrameworkColor(el.name) : undefined, + series: [{ name: applyEllipsis(el.name, 20), x: el.x, y: el.y }], + })) - }) })
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 35dff1bd-c07b-42e0-9c66-353be24e2794
📒 Files selected for processing (1)
app/components/Compare/FacetQuadrantChart.vue
Resolves #2387
This adds a quadrant chart to the compare page, to visualise comparisons along 2 axes:
The chart is displayed below the bar charts, and includes:
Data processing:
Some signals are inverted (so lower is better): size, deps, vulnerabilities
Deprecation is a hard override and forces min efficiency
The weights I have chosen can be subject to discussion, because of their arbitrary nature:
Other